Handling of errors - like unstable network - coming via SSL#89
Open
bligeti wants to merge 17 commits intofhessel:masterfrom
Open
Handling of errors - like unstable network - coming via SSL#89bligeti wants to merge 17 commits intofhessel:masterfrom
bligeti wants to merge 17 commits intofhessel:masterfrom
Conversation
In case closeConnection() is called due to SSL reported error - like session disconnection due to unstable Wifi - onClose() should be called to free up the recorded websocket handlers.
From updateBuffer() a session clean up is initiated via closeConnection() if error is detected when reading from SSL but the already removed session is not handled in other parts of the state machine.
fhessel
added
CI: Build Examples
fhessel
reviewed
Jun 21, 2020
Owner
fhessel
left a comment
fhessel
left a comment
There was a problem hiding this comment.
Thanks for providing the PR, I'll try to check it on a board asap.
However, during the code review I noticed this line (which has been there already before the PR):
size_t HTTPSConnection::readBytesToBuffer(byte* buffer, size_t length) {
Using size_t instead of ssize_t means that no underlying SSL error will ever be reported to the caller. That alone will cause trouble when using readBytesToBuffer(). The same goes for passing the return value of recv() in the basic HTTPConnection variant.
In particular, this means that the else branch in HTTPConnection.cpp:195..212 will never be reached with an int readReturnCode being set based on a size_t.
if (readReturnCode > 0) {
// ...
} else if (readReturnCode == 0) {
// ...
} else {
// dead code
}So I would be curious if you ever saw that added SSL_error=... message in your logs?
Co-authored-by: Frank Hessel <frnkhessel@googlemail.com>
Co-authored-by: Frank Hessel <frnkhessel@googlemail.com>
KaloNK
added a commit
to KaloNK/esp32_https_server
that referenced
this pull request
Nov 29, 2021
Merge fhessel#89 from fhessel/esp32_https_server
KaloNK
added a commit
to KaloNK/esp32_https_server
that referenced
this pull request
Nov 29, 2021
* Trigger wsHander onClose from closeConnection() In case closeConnection() is called due to SSL reported error - like session disconnection due to unstable Wifi - onClose() should be called to free up the recorded websocket handlers. * Handle error triggered by closed sessions From updateBuffer() a session clean up is initiated via closeConnection() if error is detected when reading from SSL but the already removed session is not handled in other parts of the state machine. * Get detailed SSL_read error * Update src/HTTPSConnection.cpp Co-authored-by: Frank Hessel <frnkhessel@googlemail.com> * Update src/HTTPConnection.cpp Co-authored-by: Frank Hessel <frnkhessel@googlemail.com> * correct the copy-paste error Co-authored-by: bligeti <64783210+bligeti@users.noreply.github.com> Co-authored-by: Frank Hessel <frnkhessel@googlemail.com>
gb-123-git
added a commit
to gb-123-git/esp32_https_server
that referenced
this pull request
Jan 23, 2022
May help in crash fix for websockets.
gb-123-git
added a commit
to gb-123-git/esp32_https_server
that referenced
this pull request
Jan 23, 2022
Update as mentioned in issue fhessel#89 for possible crash fix on websockets.
khoih-prog
added a commit
to khoih-prog/ESP32_HTTPS_Server
that referenced
this pull request
Sep 27, 2022
Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157
This was referenced Sep 27, 2022
Closed
khoih-prog
added a commit
to khoih-prog/ESP32_HTTPS_Server
that referenced
this pull request
Sep 27, 2022
1. Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157 2. Update examples and `README.md`
khoih-prog
added a commit
to khoih-prog/ESP32_HTTPS_Server
that referenced
this pull request
Sep 27, 2022
1. Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157 2. Update examples and `README.md`
khoih-prog
added a commit
to khoih-prog/ESP32_HTTPS_Server
that referenced
this pull request
Sep 27, 2022
1. Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157 2. Update examples and `README.md`
gb-123-git
added a commit
to gb-123-git/esp32_https_server
that referenced
this pull request
Nov 1, 2023
added 7 commits
September 29, 2024 16:12
… request to non-WebSocket node.
added 4 commits
October 3, 2024 00:17
…s (like curl) who ignores http 401 will connect to websocket. This fix checks if the handshake was really done (HTTP 101 was replied).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Possible fix for #85
The trigger of this problem can be network disconnections or similar errors coming via SSL.
In HTTPConnection::updateBuffer() closeConnection() is called when error is detected from SSL to clean up but the possibility of already cleaned up session is not handled in HTTPConnection::pendingBufferSize() and in HTTPConnection::loop().
Also in such case WSHandler::onClose() is not called therefore the recorded handlers (like in the websocket example) are not cleaned up.